[Feature] sse 연결 시점을 앱 시작 시점으로 변경 및 react-native-svg 라이브러리 업그레이드#90
[Feature] sse 연결 시점을 앱 시작 시점으로 변경 및 react-native-svg 라이브러리 업그레이드#90
react-native-svg 라이브러리 업그레이드#90Conversation
Summary of ChangesHello @dioo1461, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 앱의 전반적인 성능과 사용자 경험을 개선하기 위한 중요한 변경 사항들을 포함합니다. SSE 연결 방식을 최적화하여 리소스 사용을 줄이고, 특정 플랫폼에서의 라이브러리 성능 문제를 해결합니다. 또한, 그룹 관리 로직을 개선하고 채팅 메시지 알림 시스템을 더욱 견고하게 만듭니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 두 가지 주요 변경 사항을 포함합니다: SSE 연결 관리 방식을 앱 전역 Provider로 중앙화하여 개선하고, 그룹 생성 시 공개/비공개 설정 로직을 변경했습니다. 또한 react-native-svg 라이브러리를 업그레이드했습니다.
전반적으로 코드 변경은 긍정적이며, 특히 SSE 연결 로직을 SSEProvider로 분리하여 재사용성을 높이고 관련 컴포넌트의 코드를 크게 단순화한 점이 좋습니다.
한 가지 개선점을 제안했습니다. SSEProvider 내에 중복된 useEffect 로직이 있어 이를 제거하여 코드를 더 명확하고 간결하게 만들 수 있습니다. 자세한 내용은 개별 코멘트를 참고해주세요.
src/SSEProvider.tsx
Outdated
| const { connect, disconnect } = useChatRoomListSse({ | ||
| onInitMessage: (payload) => { | ||
| payload.rooms?.forEach(r => { | ||
| const count = Math.max(0, r.unread ?? 0); | ||
| store.setCount(r.roomId, count); | ||
| }); | ||
| }, | ||
| onNewMessage: (payload) => { | ||
| store.inc(payload.roomId); | ||
| }, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| connect(); | ||
| return () => disconnect(); | ||
| }, [connect, disconnect]); |
There was a problem hiding this comment.
useChatRoomListSse 훅은 내부적으로 useEffect를 사용하여 accessToken의 변경에 따라 자동으로 SSE 연결 및 해제를 관리합니다. 따라서 SSEProvider 컴포넌트에서 useEffect를 사용하여 수동으로 connect와 disconnect를 호출하는 것은 중복된 로직입니다. 이로 인해 코드가 복잡해지고 잠재적인 혼란을 야기할 수 있습니다.
useChatRoomListSse 훅이 연결 생명주기를 자체적으로 처리하도록 하고, SSEProvider에서는 불필요한 useEffect와 connect, disconnect 변수 선언을 제거하는 것이 좋습니다.
| const { connect, disconnect } = useChatRoomListSse({ | |
| onInitMessage: (payload) => { | |
| payload.rooms?.forEach(r => { | |
| const count = Math.max(0, r.unread ?? 0); | |
| store.setCount(r.roomId, count); | |
| }); | |
| }, | |
| onNewMessage: (payload) => { | |
| store.inc(payload.roomId); | |
| }, | |
| }); | |
| useEffect(() => { | |
| connect(); | |
| return () => disconnect(); | |
| }, [connect, disconnect]); | |
| useChatRoomListSse({ | |
| onInitMessage: (payload) => { | |
| payload.rooms?.forEach(r => { | |
| const count = Math.max(0, r.unread ?? 0); | |
| store.setCount(r.roomId, count); | |
| }); | |
| }, | |
| onNewMessage: (payload) => { | |
| store.inc(payload.roomId); | |
| }, | |
| }); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2d293ab21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| groupId={group.groupId} | ||
| groupName={group.groupName} | ||
| isPublic={group.isOpen} | ||
| isPublic={!group.isApprovalRequired} | ||
| participantImageUrls={[leaderImageUrl, ...otherImageUrls]} |
There was a problem hiding this comment.
GetGroupsResponse exposes both isOpen and isApprovalRequired as independent fields (see src/features/groups/model/groups.ts), so treating isPublic as !isApprovalRequired can be wrong when a group is publicly searchable but still requires approval. This also makes the list view disagree with the detail view, which still uses params.isOpen. In those cases, groups will be labeled private in the list even though they are open. Prefer using group.isOpen (or whatever the API contract defines) for public/private status.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR refactors the SSE (Server-Sent Events) connection architecture to establish a single persistent connection at app startup, upgrades the react-native-svg library to address Windows performance issues, and adjusts the group creation default settings for publicity.
Changes:
- Centralized SSE connection management by introducing an SSEProvider that connects once at app startup and maintains a global store for chat room unread counts
- Upgraded
react-native-svgfrom 15.11.2 to 15.15.2 to fix Windows performance issues - Changed the default group publicity setting from private (approval required) to public (no approval required)
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SSEProvider.tsx | New provider component that manages a single SSE connection and exposes a global store for chat room unread counts using useSyncExternalStore pattern |
| src/features/chat/sse/useChatCount.ts | New hook that subscribes to the global SSE store to get unread count for a specific chat room |
| src/App.tsx | Wraps the app with SSEProvider to establish SSE connection at startup |
| src/screens/group/groupDetail/GroupDetailScreen/index.tsx | Simplified by removing local SSE connection management and using the new useChatCount hook |
| src/screens/group/CreateGroupScreen/index.tsx | Changed default value for isApprovalRequired from true to false, making groups public by default |
| src/screens/group/CreateGroupScreen/components/GroupPublicityInput.tsx | Updated label and hint logic to reflect the new semantics where the checkbox means "make private" rather than "make public" |
| src/screens/bottomTab/GroupsScreen/components/JoinedGroupPanel/index.tsx | Changed logic to determine if group is public from group.isOpen to !group.isApprovalRequired |
| src/locales/ko/createGroup.json | Updated label from "그룹 공개하기" (make group public) to "비공개 그룹으로 만들기" (make group private) |
| src/hooks/useChatRoomListSse/index.ts | Removed TODO comment about UX improvements, as it's being addressed by this PR |
| package.json, package-lock.json | Upgraded react-native-svg from 15.11.2 to 15.15.2 |
Comments suppressed due to low confidence (1)
src/screens/group/CreateGroupScreen/index.tsx:49
- The form is missing the
isOpenfield, but theCreateGroupRequesttype requires it. The form only provides values forname,tag,maxMember, andisApprovalRequired. This will cause the submitted data to be incomplete and may result in API validation errors. You should either add a Form element for theisOpenfield or set a default value using theuseFormPropsprop on the Form component.
<Form gap={20}>
<GroupNameInput defaultValue='' formName='name' />
<GroupTagInput defaultValue='' formName='tag' />
<GroupMaxMemberInput defaultValue={20} formName='maxMember' />
<GroupPublicityInput defaultValue={false} formName='isApprovalRequired' />
<Form.SubmitButton onSubmit={onSubmit}>
{(submit, disabled) => (
<Button.Basic disabled={disabled} onPress={submit}>
{tCreateGroup}
</Button.Basic>
)}
</Form.SubmitButton>
</Form>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hint={!isPublic ? tPublicityDescriptionPublic : tPublicityDescriptionPrivate} | ||
| label={tMakePublicityPrivate} |
There was a problem hiding this comment.
The hint logic appears to be inverted. The variable isPublic is watching isApprovalRequired (which represents whether the group is private). When the checkbox is unchecked (isApprovalRequired=false, group is public), isPublic is false, so !isPublic is true and it shows the public description, which is correct. However, the variable name isPublic is very misleading here since it actually holds the value of isApprovalRequired. This makes the code harder to understand and maintain.
src/SSEProvider.tsx
Outdated
| return <SseContext.Provider value={store}>{children}</SseContext.Provider>; | ||
| }; | ||
|
|
||
| export const useSSEContext = () =>useSafeContext(SseContext); |
There was a problem hiding this comment.
Missing space between function name and parentheses. Should be useSSEContext = () => useSafeContext with a space after the arrow.
| export const useSSEContext = () =>useSafeContext(SseContext); | |
| export const useSSEContext = () => useSafeContext(SseContext); |
✨ 구현 기능 명세
react-native-svg라이브러리를 업그레이드하였습니다.✅ PR Point
😭 어려웠던 점